Prototype targets.merge function #1826

ptodev · 2024-10-04T10:04:06Z

I hope this can enable users to decorate prometheus.exporter metrics with labels from discovery components.

The config could look like this:

discovery.kubernetes "default" {
  role = "service"
}

prometheus.exporter.unix "default" {}

// Probably we'd need some discovery.relabel to make sure both targets have a common key/val to serve as condition of joining

prometheus.scrape "demo" {
  targets    = targets.merge(prometheus.exporter.unix.default.targets, discovery.kubernetes.default.targets, ["__meta_kubernetes_service_cluster_ip"])
  forward_to = [prometheus.remote_write.mimir.receiver]
}

prometheus.remote_write "mimir" {
  endpoint {
    url = "https://prometheus-prod-05-gb-south-0.grafana.net/api/prom/push"

    basic_auth {
      username = ""
      password = ""
    }
  }
}

Which issue(s) this PR fixes

Fixes #1443

docs/sources/reference/stdlib/map.md

mattdurham · 2024-10-04T12:58:46Z

I wonder on the naming, the name makes me feel like its very SQL like and should behave like a SQL inner join.

ptodev · 2024-10-09T09:51:12Z

I wonder on the naming, the name makes me feel like its very SQL like and should behave like a SQL inner join.

@mattdurham Good point - I renamed it from "inner_join" to "merge". I also changed the namespace from "map" to "targets", because I realised that even "map" is not an appropriate namespace for this function as it operates on arrays of maps.

ptodev · 2024-10-09T09:56:01Z

docs/sources/reference/stdlib/targets.md

+
+* The first two inputs are a of type `list(map(string))`. The keys of the map are strings. 
+  The value for each key could have any Alloy type such as a string, integer, map, or a capsule.
+* The third input is an array containing strings. The strings are the keys whose value has to match for maps to be joined.


Currently, the "third input" assumes both targets have a label with the same value. I wonder if it's worth changing the third input to be an array of arrays, where we can set different label names in the LHS and RHS targets. E.g.:

[["lhs_lbl_name"], ["rhs_lbl_name"], ["lhs_lbl_name_2"], ["rhs_lbl_name_2"]]

This could reduce the amount of relabels a user would have to do. But I don't know if it's really worth the extra complexity. The users probably have to relabel anyway.

mattdurham · 2024-10-09T12:35:25Z

syntax/vm/vm_stdlib_test.go

+		{
+			// Basic case. No conflicting key/val pairs.
+			"targets.merge",
+			`targets.merge([{"a" = "a1", "b" = "b1"}], [{"a" = "a1", "c" = "c1"}], ["a"])`,


What is the result for invalid input? Imagine the first is a valid map list but the second is nil/invalid type/etc? I imagine it would return the map list, whereas if it is the reverse it would return a blank list?

There is an error if the input is in valid:

docs/sources/reference/stdlib/targets.md

mattdurham · 2024-10-09T12:54:20Z

Overall looks good! Though the complexity of this feels like it borders on a component, which means we could use the live debugging and mark as experimental. Since this has become specific to targets thoughts to add it to relabel.discovery?

ptodev · 2024-10-24T15:48:54Z

docs/sources/reference/stdlib/targets.md

+
+## targets.merge
+
+The `targets.inner_join` function allows you to join two arrays containing maps if certain keys have matching values in both maps.


Suggested change

The `targets.inner_join` function allows you to join two arrays containing maps if certain keys have matching values in both maps.

The `targets.merge` function allows you to join two arrays containing maps if certain keys have matching values in both maps.

I need to rename to targets.merge here and in all other places in the doc.

ptodev · 2024-10-31T11:29:04Z

Overall looks good! Though the complexity of this feels like it borders on a component, which means we could use the live debugging and mark as experimental. Since this has become specific to targets thoughts to add it to relabel.discovery?

@mattdurham The main reasons for having it as an std lib function are:

It is technically not specific to targets. You could have any lists of maps and the function could still work. I namespaced it as "targets", just because I couldn't think of a better name.
It's probably more performant as stdlib than as a component. This is just a guess and I'm not sure what the benchmarks would be in reality.

I do agree that there would be benefits to the component approach:

Works with the UI - not just live debugging, but the actual UI that shows inputs.
We can gate it behind the "experimental" cmd flag.
It's easier for people who use discovery components to find it.

We could do a few things to make the std function a more compelling option:

Rename this function to something that doesn't include "targets" in the name. For example, map.merge()? Technically, the inputs are not maps because we're merging arrays of maps... but that's probably fine.
We'd have to rename the objects type to "maps". I asked the team about this a while back, and no one objected. It is already documented as "map" in the component reference docs.
I could add examples of how to use the "targets.merge"/"map.merge" function with non-discovery functionality. For example, we could get two maps from encoding.from_json and merge them.
It just dawned on me that we can also do filtering with this too. E.g. map.merge(arr_w_maps, [{"val" = "a"}], ["val"]) will only pass through the maps in arr_w_maps which have "val" = "a". It's a bit weird that you can do filtering with a function called merge, but I can't think of a better alternative right now 😄 I could include an example for that too.

In the long term, we could also:

Make the UI work better with std lib functions.
Make it possible to gate std lib features behind an "experimental" command line argument.

cc @thampiotr he made the original suggestion to use std lib.

ptodev · 2024-10-31T19:34:33Z

Maybe map.combine might be better than map.merge, since the function makes a new map for each combination of LHS map and a RHS map.

ptodev commented Oct 4, 2024

View reviewed changes

docs/sources/reference/stdlib/map.md Outdated Show resolved Hide resolved

docs/sources/reference/stdlib/map.md Outdated Show resolved Hide resolved

ptodev mentioned this pull request Oct 4, 2024

Document a new "join" block in discovery.relabel #1541

Closed

4 tasks

wildum reviewed Oct 4, 2024

View reviewed changes

docs/sources/reference/stdlib/map.md Outdated Show resolved Hide resolved

wildum reviewed Oct 4, 2024

View reviewed changes

docs/sources/reference/stdlib/map.md Outdated Show resolved Hide resolved

ptodev force-pushed the ptodev/inner-join-func branch from 64767f2 to b248d1c Compare October 9, 2024 09:47

ptodev changed the title ~~Prototype map.inner_join function~~ Prototype targets.merge function Oct 9, 2024

ptodev commented Oct 9, 2024

View reviewed changes

mattdurham reviewed Oct 9, 2024

View reviewed changes

docs/sources/reference/stdlib/targets.md Outdated Show resolved Hide resolved

ptodev force-pushed the ptodev/inner-join-func branch from b248d1c to 2c5dea2 Compare October 17, 2024 12:43

ptodev commented Oct 24, 2024

View reviewed changes

ptodev added 3 commits October 31, 2024 13:37

Prototype map.inner_join function

8b6920d

Rename to "targets.merge", remove unnecessary params

dc0a3cd

Add failure tests

0ef415c

ptodev force-pushed the ptodev/inner-join-func branch from 2c5dea2 to 0ef415c Compare October 31, 2024 19:18

Delete incorrect comment

955fc04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prototype targets.merge function #1826

Prototype targets.merge function #1826

ptodev commented Oct 4, 2024 •

edited

Loading

mattdurham commented Oct 4, 2024

ptodev commented Oct 9, 2024

ptodev Oct 9, 2024

mattdurham Oct 9, 2024

ptodev Oct 31, 2024

mattdurham commented Oct 9, 2024

ptodev Oct 24, 2024

ptodev commented Oct 31, 2024

ptodev commented Oct 31, 2024


		## targets.merge

		The `targets.inner_join` function allows you to join two arrays containing maps if certain keys have matching values in both maps.

	The `targets.inner_join` function allows you to join two arrays containing maps if certain keys have matching values in both maps.
	The `targets.merge` function allows you to join two arrays containing maps if certain keys have matching values in both maps.

Prototype targets.merge function #1826

Are you sure you want to change the base?

Prototype targets.merge function #1826

Conversation

ptodev commented Oct 4, 2024 • edited Loading

Which issue(s) this PR fixes

mattdurham commented Oct 4, 2024

ptodev commented Oct 9, 2024

ptodev Oct 9, 2024

Choose a reason for hiding this comment

mattdurham Oct 9, 2024

Choose a reason for hiding this comment

ptodev Oct 31, 2024

Choose a reason for hiding this comment

mattdurham commented Oct 9, 2024

ptodev Oct 24, 2024

Choose a reason for hiding this comment

ptodev commented Oct 31, 2024

ptodev commented Oct 31, 2024

ptodev commented Oct 4, 2024 •

edited

Loading